fix(native): cut circular variable resolution instead of blowing the stack - #422

Problem

CSS Variables Level 1 §3 is explicit about reference cycles: "if there is a cycle in the dependency graph, all the custom properties in the cycle must compute to their guaranteed-invalid value" — which makes the consuming declaration invalid at computed-value time and leaves the property unset. A well-defined, non-fatal outcome.

The native runtime instead recurses until the JS stack is exhausted and throws out of render. Measured on main (f70c402) through @testing-library/react-native:

registerCSS(`.themed { --a: var(--b); --b: var(--a); width: var(--a) }`, { inlineVariables: false });
render(<View className="themed" />);

RangeError: Maximum call stack size exceeded
  at resolve (src/native/styles/variables.ts:51:12)
  at resolveValue (src/native/styles/resolve.ts:105:16)
  at resolveValue (src/native/styles/resolve.ts:109:27)
  at resolve (src/native/styles/variables.ts:51:12)
  … repeating

In an app that reaches whatever error boundary sits above the tree, so one cyclic custom property replaces a screen with a fallback.

Why it is reachable from ordinary CSS

The compiler has a working cycle guard of its own — flattenVar's seen set in the inlineVariables pass — which is why this is not fired by every stylesheet. But that pass only folds a custom property declared exactly once (src/compiler/inline-variables.ts: if (info.count !== 1) vars.delete(name)), so the same CSS with default options is folded away before the runtime sees it. Declare the token twice — a base value plus a prefers-color-scheme override, which is the ordinary shape of themed CSS — and it skips the inliner and reaches the resolver.

Measured on main, all three with the same class:

The third row is the point: no compiler option changed, no exotic input, just a theme token and a cycle someone did not notice.

Cycles split across an ancestor/descendant pair reach it too, because inherited variables resolve through the same function — and that is the shape the tests use, since a single-definition variable is folded away before the runtime sees it.

Root cause — the guard was dead code

varResolver read its guard out of options with a default, and never wrote it back:

const {
  …
  variableHistory = new Set(),   // ← a fresh Set on every call
} = options;

if (variableHistory.has(name)) return;   // ← therefore always false
variableHistory.add(name);               // ← mutates a Set that is discarded

resolve closes over the same options object, whose .variableHistory stays undefined, so every nested varResolver allocated a new empty Set. There are exactly four variableHistory references in src/ on main — the optional field on ResolveValueOptions, and the destructure / .has / .add above. Nothing anywhere assigns it.

A second hole sits beside it: the if (name in variables) early return recurses before .add is reached. That is the branch a descendant takes, and therefore the branch the recursion runs through, so even a working set would have been bypassed.

Fix

Two changes, both in varResolver.

1. The set lives on options — const namesBeingResolved = (options.namesBeingResolved ??= new Set<string>()) — so every nested resolve below shares it.
2. It holds the names on the CURRENT path, not every name ever seen: added before any of the name's values are resolved, removed again in a finally. The name in variables branch moves inside the try.

Both halves are load-bearing, and each is pinned from its own side. Registering the name cuts the cycle. Removing it per frame — rather than emptying the stack — is what keeps a name readable again once its own resolution has finished, which a name read twice within ONE declaration needs (box-shadow: var(--c) 1px 1px, var(--c) 2px 2px).

Only the within-one-declaration case depends on it. applyDeclarations builds a fresh options object at each of its three resolveValue call sites in calculate-props.ts — the transform, delayed and plain arms — so two declarations never share a stack in the first place. Measured: hoisting a single shared options object across every declaration leaves the whole suite at the exact baseline, 1058 passed / 3 failed. So the finally is not there to un-block a second declaration; it is there so that one declaration's second read, and one value's second branch, are not mistaken for re-entry.

The field is renamed variableHistory → namesBeingResolved, matching the local it feeds and what it holds. ResolveValueOptions is internal to src/native/styles/ and the field is re-exported from no entry point.

The diff looks larger than it is — a good part of it is the four existing lookup tiers moving one indentation level into the try, unchanged.

Which plane

Native runtime (src/native/styles/variables.ts), alone. varResolver is referenced only from src/native/styles/resolve.ts, and there is no variable resolution anywhere under src/web — on web the CSS is served to the browser and the cycle rule above is the browser's to implement. So there is no web mirror to write.

Tests

10 cases in src/__tests__/native/variables.test.tsx, describe("circular variables"). 6 of them fail with RangeError: Maximum call stack size exceeded against the unfixed varResolver — that is the reproduction. Substituting main's varResolver back in under these tests gives 1052 passed, 9 failed against this branch's 1058 passed, 3 failed — 6 new reds plus the 3 pre-existing Windows failures below. The 6 are the four census rows and both public entry points.

• A four-row census driven by test.each behind a not-empty sentinel: a variable whose value is itself, a variable reached again through a fallback, two variables that name each other, and one name re-entered from two branches of a single value.
• Both public entry points that recurse without the guard: useUnstableNativeVariable, and VariableContextProvider, whose value type admits a var() reference.
• Two guards that the cut is not over-broad, green on main too: a variable read twice in ONE declaration (a two-shadow box-shadow) is not mistaken for a cycle, and a long non-circular chain still resolves.
• The compiler's own cycle guard, which nothing in the repo covered: .child { --z: var(--z); width: var(--z) } with --z declared once never reaches the runtime, so flattenVar's seen set is what stops it. That guard decides whether the runtime guard is reached at all, and until now it had no test anywhere.

No test asserts a throw. Every case asserts a successful render, so the crash is proven red-to-green rather than pinned as a toThrow.

Every row is falsifiable

A census row asserting only toStrictEqual({}) cannot tell "the cycle was cut" from "resolution no longer works" — an empty style is what both produce. So every row reads a non-cyclic --unrelated beside the cycle, and a row can only pass while resolution still works. Every name in a cycle is also declared twice, because a variable declared exactly once is substituted into its readers and never reaches the runtime resolver these rows exist to exercise.

Measured by mutating the guard and running the full suite. Baseline is 3 failed — the pre-existing Windows-only babel-plugin-tester cases, below.

The third and fourth rows pin the finally from both sides: removing too little reddens the box-shadow, removing too much reddens the two-branch row, and neither mutation alone reaches the other's test.

Suite

Test Suites: 2 failed, 4 skipped, 53 passed, 55 of 59 total
Tests:       3 failed, 21 skipped, 1058 passed, 1082 total

numRuntimeErrorTestSuites: 0. main (f70c402) is 1048 passed, 1072 total on the same machine, so this is +10 tests and no new failures. The 3 are react-native › plugin › 7, react-native-web › plugin › 6 and › 17 — babel-plugin-tester cases over an unrewritten relative require("../View"), which fail identically at every ref on Windows. That count is stable on a warm cache; a cold or loaded run adds a tail of first-in-file 5000ms timeouts that are not this branch's either. yarn typecheck and yarn lint exit 0.

KNOWN LIMITS

The cut is not always the spec's outcome — sometimes the property keeps a truncated value. The cut returns undefined, and resolveValue's descriptor-array branch filters undefined out of the array and keeps the surviving siblings, so a cycle that is only part of a larger value leaves the rest behind rather than invalidating the declaration. Measured on this branch:

The two-branch census row pins the second of these ({ color: [], opacity: 0.5 }) so the shape is at least recorded rather than incidental. Every one of them is a bounded, renderable value instead of a crash, which is the change this PR is claiming; making them unset is a separate change to how resolveValue treats a missing piece of a descriptor array, and it would move values that have nothing to do with cycles.

I have not verified whether color: [] / width: [1] / transform: [{}] are tolerated or fatal in React Native's own layer — the measurements above are jest, not a device.

The guard bounds cycles only. A non-circular chain resolves to great depth, but a long enough one still exhausts the JS stack and throws RangeError. The guard neither helps nor hurts there — a chain never re-enters a name, so it never reaches the cut — and this PR does not claim to fix it. The ceiling is a property of the JS stack rather than a constant this library owns, so I have deliberately not written a number down; the test pins that a long chain resolves, not how long.

A cyclic variable swallows its reader's fallback: var(--cyclic, blue) yields {} where CSS says blue. This is pre-existing and not cycle-specific. varResolver's first arm is presence-keyed — if (name in variables) return resolve(variables[name]) — so any inherited name that is present and resolves to nothing takes the reader's fallback with it. Measured on this branch: a declared-but-unresolvable non-cyclic variable swallows the fallback, a cyclic one swallows it, and a never-declared name correctly takes it. Those are main's results too — this PR moves that arm one indentation level into the try and changes nothing else about it. #431 documents this exact shape in its own body, but its fix is at the two record builders that plant a key holding undefined; it does not touch that early return, so landing #431 will not fix the CSS-declared case.

The keyframes boundary is an open question, not a fix. shorthands/animation.ts re-enters calculateProps with a fresh options object, so the resolution stack does not cross into a keyframe pass. Three attempts to construct a cycle that is reachable through it all rendered cleanly — the animation name resolves and its frame pops before the keyframe pass runs, leaving no live frame to re-enter. With no reproduction I have not written a fix; there is a comment marking the boundary so the next person starts from what is known.

Nothing warns. A stylesheet with an accidental cycle silently loses a declaration where it previously lost the screen. That is strictly better, but if you would like a dev-mode warning at the cut point it is a two-line addition and I will add it.

The compile-time guard and this one remain two guards. flattenVar's seen set and this resolution stack solve the same problem at different times, and neither knows about the other. Unifying them is not possible as things stand — the compiler can only see cycles inside the properties it is allowed to fold — so this is a note rather than a plan. Both now have a test.

Overlaps with open PRs, measured with a three-way git merge-file of each PR head against the shared base f70c402:

#412 (fix/non-inheriting-custom-properties) conflicts, and the substantive risk is larger than the textual one. It adds two rungs to src/native/styles/variables.ts — a nonInheritedVariables gate around the rootVariables lookup, and a new registeredInitialValues lookup after it — in exactly the region this PR re-indents into its try. One conflict hunk, resolvable by hand in a minute. What a hand-merge must get right is that both of #412's rungs land INSIDE the try, alongside the four existing tiers. Land them after the finally and that tier resolves outside the resolution stack, unguarded, with no test on either branch that would notice.

#431 (fix/vars-undefined-key) conflicts trivially, in src/__tests__/native/variables.test.tsx and nowhere else: both PRs add an import from react-native-css/native at the top of the file, this one for useUnstableNativeVariable and VariableContextProvider, #431 for VariableContextProvider alone. One hunk, one merged import statement.

#413 (fix/single-definition-inliner-scope) and #389 (fix/scale-percentage) auto-merge clean today. #413 shares src/__tests__/native/variables.test.tsx and src/compiler/inline-variables.ts, #389 shares src/native/styles/resolve.ts; all three files merge without a conflict.

#413 is also related in substance, in a way that helps: it scopes the single-definition inliner to its declaring block, which means more custom properties survive to the runtime resolver. Landing it without this one widens the surface on which the crash is reachable.